-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Saving sms conversations into crm task #72
Conversation
@Override | ||
public EnvironmentConfig.CRMFieldDefinitions getFieldDefinitions() { | ||
return this.env.getConfig().salesforce.fieldDefinitions; | ||
} | ||
|
||
protected void setTaskFields(SObject task, CrmTask crmTask) { | ||
task.setField("Id", crmTask.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's technically a setId method. But, I vaguely remember the API might reject this if the Id is explicitly set as null, so might need a null/empty check around this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added null check
.collect(Collectors.joining(",")); | ||
} | ||
|
||
crmTask.assignTo = "0057Q000000M8FzQAK"; // me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shift to env.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
crmTask.assignTo = "0057Q000000M8FzQAK"; // me | ||
crmTask.status = CrmTask.Status.IN_PROGRESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPEN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
import java.util.List; | ||
|
||
public class CrmActivity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little confused about the difference between CrmActivity, CrmTask, and why we need both. Lean into that for me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VSydor Thoughts on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brmeyer The idea is that crmActivity is a sub-type of crmTask that has crmActivityType (conversation or email) and conversationId fields. However, when converting to crmTask - these 2 fields just form the subject of the task (type + ":" + conversationId) so technically it's the same thing.
should I work directly with task and remove crmActivity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without CrmActivity will look like this:
#98
Please select the one that fits best
@@ -172,6 +173,9 @@ default void updateContact(OpportunityEvent opportunityEvent) throws Exception { | |||
Optional<CrmUser> getUserById(String id) throws Exception; | |||
Optional<CrmUser> getUserByEmail(String email) throws Exception; | |||
String insertTask(CrmTask crmTask) throws Exception; | |||
String updateTask(CrmTask crmTask) throws Exception; | |||
String upsertActivity(CrmActivity crmActivity) throws Exception; | |||
Optional<CrmActivity> getActivityByTypeAndConversationId(CrmActivity.Type type, String conversationId) throws Exception; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this more generic? by type and an abstract notion of an ID? External ID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to external id
333e589
to
b49c48a
Compare
2c60802
to
ee7d368
Compare
…roller for manual testing
ee7d368
to
907f480
Compare
Closing in favor of: |
…roller for manual testing